Skip to content

Notification Rule Processing Engine #6

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
akshayutture-augment wants to merge 1 commit into
base: notification-rule-baseline
Choose a base branch
from
Open

Notification Rule Processing Engine #6

akshayutture-augment wants to merge 1 commit into from

Conversation

@akshayutture-augment
Copy link

@akshayutture-augment akshayutture-augment commented Nov 14, 2025

@konrad147 @soniaAguilarPeiron
Alerting: Remove ruler from alert list view2 (#106778)
3e6d620 
* wip

* Add working actions for GMA rules based on Prom-only API

* Remove Ruler-loader related code for Grafana rules

Co-authored-by: Sonia Augilar <[email protected]>

* Remove outdated tests

* add some comments

* remove commented code

* remove showLocation property

* Add missing mocks in tests

* Add showLocation to GrafanaRuleListItem, improve useAbilities, address PR feedback

* Enhance GrafanaGroupLoader tests: Add permission checks and More button functionality

- Introduced user permission grants for alerting actions in tests.
- Added tests for rendering the More button with action menu options.
- Verified that each rule has its own action buttons and handles permissions correctly.
- Ensured the edit button is not rendered when user lacks edit permissions.
- Confirmed the correct menu actions are displayed when the More button is clicked.

* Update translations

---------

Co-authored-by: Sonia Aguilar <[email protected]>
Co-authored-by: Sonia Augilar <[email protected]>
greptile-apps[bot]
greptile-apps bot reviewed Nov 14, 2025
View reviewed changes
Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.

@akshayutture-augment
Copy link
Author

akshayutture-augment commented Nov 14, 2025

@greptileai

@greptile-apps
Copy link

greptile-apps bot commented Nov 14, 2025

Greptile Overview

Confidence Score: 4/5

  • This PR is safe to merge with careful monitoring of permissions behavior in production
  • The refactoring is well-structured with comprehensive test coverage, but introduces a significant architectural change in how permissions are checked. The dual-source logic (Ruler OR Prometheus) provides good backward compatibility, though the permission checking logic in useIsGrafanaPromRuleEditable needs validation in production to ensure folder permissions work correctly for all scenarios.
  • Pay close attention to public/app/features/alerting/unified/hooks/useAbilities.ts - verify that the new useIsGrafanaPromRuleEditable correctly implements permission checks equivalent to the Ruler-based approach

Important Files Changed

File Analysis

Filename Score Overview
public/app/features/alerting/unified/hooks/useAbilities.ts 4/5 Added new hooks useGrafanaPromRuleAbility and useGrafanaPromRuleAbilities to check permissions using GrafanaPromRuleDTO instead of RulerRuleDTO, enabling rule list view to work without Ruler API calls. Includes skipToken symbol for conditional hook execution.
public/app/features/alerting/unified/rule-list/GrafanaGroupLoader.tsx 5/5 Removed Ruler API dependency - now only fetches from Prometheus API. Simplified logic by removing rule matching between Ruler and Prometheus responses, eliminating transient state handling for creating/deleting rules.
public/app/features/alerting/unified/rule-list/GrafanaRuleListItem.tsx 5/5 New component that renders a single Grafana rule using only GrafanaPromRuleDTO, replacing GrafanaRuleLoader. Handles alerting, recording, and unknown rule types.
public/app/features/alerting/unified/rule-list/components/RuleActionsButtons.V2.tsx 4/5 Updated to support both RulerRuleDTO and GrafanaPromRuleDTO for ability checks. Uses useGrafanaPromRuleAbility hook to check edit permissions when promRule is available, falling back to useRulerRuleAbility for backward compatibility.
public/app/features/alerting/unified/components/rule-viewer/AlertRuleMenu.tsx 4/5 Refactored to use useRulerRuleAbilities and useGrafanaPromRuleAbilities hooks (plural versions) to batch ability checks. Combines permissions from both Ruler and Prometheus rules using OR logic.

Sequence Diagram

sequenceDiagram
    participant User
    participant RuleListView
    participant GrafanaGroupLoader
    participant PrometheusAPI
    participant RulerAPI
    participant GrafanaRuleListItem
    participant RuleActionsButtons
    participant AlertRuleMenu
    participant useAbilities

    Note over RuleListView,useAbilities: New Flow (Post-PR)
    User->>RuleListView: View alert rules
    RuleListView->>GrafanaGroupLoader: Load group
    GrafanaGroupLoader->>PrometheusAPI: useGetGrafanaGroupsQuery()
    PrometheusAPI-->>GrafanaGroupLoader: GrafanaPromRuleDTO[]
    
    loop For each rule
        GrafanaGroupLoader->>GrafanaRuleListItem: Render rule with promRule only
        GrafanaRuleListItem->>RuleActionsButtons: Pass promRule
        RuleActionsButtons->>useAbilities: useGrafanaPromRuleAbility(promRule)
        useAbilities->>useAbilities: Check folder permissions
        useAbilities-->>RuleActionsButtons: [supported, allowed]
        RuleActionsButtons->>AlertRuleMenu: Pass promRule
        AlertRuleMenu->>useAbilities: useGrafanaPromRuleAbilities(promRule)
        useAbilities-->>AlertRuleMenu: Ability[]
        AlertRuleMenu-->>GrafanaRuleListItem: Menu with permissions
    end

    Note over RuleListView,useAbilities: Old Flow (Pre-PR - REMOVED)
    Note over GrafanaGroupLoader,RulerAPI: Previously called:<br/>useGetGrafanaRulerGroupQuery()<br/>and matched rules
Loading

greptile-apps[bot]
greptile-apps bot reviewed Nov 14, 2025
View reviewed changes
Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

14 files reviewed, no comments

Edit Code Review Agent Settings | Greptile

@github-advanced-security
Copy link

github-advanced-security bot commented Nov 15, 2025

This pull request sets up GitHub code scanning for this repository. Once the scans have completed and the checks have passed, the analysis results for this pull request branch will appear on this overview. Once you merge this pull request, the 'Security' tab will show more code scanning analysis results (for example, for the default branch). Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results. For more information about GitHub code scanning, check out the documentation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@greptile-apps greptile-apps[bot] greptile-apps[bot] left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@akshayutture-augment @konrad147